From a0878fe2b42f30183b4a37b34c3115907dbd20d4 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 23 Nov 2013 12:28:34 -0800 Subject: [PATCH] filebackend: exception handling cleanups * Split out FileBackendException class and reduced direct use of MWException Change-Id: I325c1798b6d90972c12a5dccc37989af34d857f3 --- includes/AutoLoader.php | 1 + includes/filebackend/FSFile.php | 4 ---- includes/filebackend/FileBackend.php | 21 ++++++++++++++----- includes/filebackend/FileBackendGroup.php | 16 +++++++------- .../filebackend/FileBackendMultiWrite.php | 10 ++++----- includes/filebackend/FileBackendStore.php | 17 ++++++++------- includes/filebackend/FileOp.php | 4 ++-- includes/filebackend/SwiftFileBackend.php | 13 +++++++----- 8 files changed, 49 insertions(+), 37 deletions(-) diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 6ee62e0424..820041c1aa 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -533,6 +533,7 @@ $wgAutoloadLocalClasses = array( 'FileBackendGroup' => 'includes/filebackend/FileBackendGroup.php', 'FileBackend' => 'includes/filebackend/FileBackend.php', 'FileBackendError' => 'includes/filebackend/FileBackend.php', + 'FileBackendException' => 'includes/filebackend/FileBackend.php', 'FileBackendStore' => 'includes/filebackend/FileBackendStore.php', 'FileBackendStoreShardListIterator' => 'includes/filebackend/FileBackendStore.php', 'FileBackendStoreShardDirIterator' => 'includes/filebackend/FileBackendStore.php', diff --git a/includes/filebackend/FSFile.php b/includes/filebackend/FSFile.php index 3a0aa9747d..047aefd973 100644 --- a/includes/filebackend/FSFile.php +++ b/includes/filebackend/FSFile.php @@ -37,12 +37,8 @@ class FSFile { * Sets up the file object * * @param string $path Path to temporary file on local disk - * @throws MWException */ public function __construct( $path ) { - if ( FileBackend::isStoragePath( $path ) ) { - throw new MWException( __METHOD__ . " given storage path `$path`." ); - } $this->path = $path; } diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index a6eda18897..0e9a41df41 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -126,12 +126,12 @@ abstract class FileBackend { * - parallelize : When to do file operations in parallel (when possible). * Allowed values are "implicit", "explicit" and "off". * - concurrency : How many file operations can be done in parallel. - * @throws MWException + * @throws FileBackendException */ public function __construct( array $config ) { $this->name = $config['name']; if ( !preg_match( '!^[a-zA-Z0-9-_]{1,255}$!', $this->name ) ) { - throw new MWException( "Backend name `{$this->name}` is invalid." ); + throw new FileBackendException( "Backend name `{$this->name}` is invalid." ); } $this->wikiId = isset( $config['wikiId'] ) ? $config['wikiId'] @@ -1360,7 +1360,7 @@ abstract class FileBackend { * * @param string $type One of (attachment, inline) * @param string $filename Suggested file name (should not contain slashes) - * @throws MWException + * @throws FileBackendError * @return string * @since 1.20 */ @@ -1369,7 +1369,7 @@ abstract class FileBackend { $type = strtolower( $type ); if ( !in_array( $type, array( 'inline', 'attachment' ) ) ) { - throw new MWException( "Invalid Content-Disposition type '$type'." ); + throw new FileBackendError( "Invalid Content-Disposition type '$type'." ); } $parts[] = $type; @@ -1416,8 +1416,19 @@ abstract class FileBackend { } /** + * Generic file backend exception for checked and unexpected (e.g. config) exceptions + * + * @ingroup FileBackend + * @since 1.23 + */ +class FileBackendException extends MWException { +} + +/** + * File backend exception for checked exceptions (e.g. I/O errors) + * * @ingroup FileBackend * @since 1.22 */ -class FileBackendError extends MWException { +class FileBackendError extends FileBackendException { } diff --git a/includes/filebackend/FileBackendGroup.php b/includes/filebackend/FileBackendGroup.php index 491424bf61..416fe84108 100644 --- a/includes/filebackend/FileBackendGroup.php +++ b/includes/filebackend/FileBackendGroup.php @@ -113,18 +113,18 @@ class FileBackendGroup { * Register an array of file backend configurations * * @param array $configs - * @throws MWException + * @throws FileBackendException */ protected function register( array $configs ) { foreach ( $configs as $config ) { if ( !isset( $config['name'] ) ) { - throw new MWException( "Cannot register a backend with no name." ); + throw new FileBackendException( "Cannot register a backend with no name." ); } $name = $config['name']; if ( isset( $this->backends[$name] ) ) { - throw new MWException( "Backend with name `{$name}` already registered." ); + throw new FileBackendException( "Backend with name `{$name}` already registered." ); } elseif ( !isset( $config['class'] ) ) { - throw new MWException( "Cannot register backend `{$name}` with no class." ); + throw new FileBackendException( "Backend with name `{$name}` has no class." ); } $class = $config['class']; @@ -142,11 +142,11 @@ class FileBackendGroup { * * @param string $name * @return FileBackend - * @throws MWException + * @throws FileBackendException */ public function get( $name ) { if ( !isset( $this->backends[$name] ) ) { - throw new MWException( "No backend defined with the name `$name`." ); + throw new FileBackendException( "No backend defined with the name `$name`." ); } // Lazy-load the actual backend instance if ( !isset( $this->backends[$name]['instance'] ) ) { @@ -163,11 +163,11 @@ class FileBackendGroup { * * @param string $name * @return array - * @throws MWException + * @throws FileBackendException */ public function config( $name ) { if ( !isset( $this->backends[$name] ) ) { - throw new MWException( "No backend defined with the name `$name`." ); + throw new FileBackendException( "No backend defined with the name `$name`." ); } $class = $this->backends[$name]['class']; diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php index b3c46c6121..612b19b188 100644 --- a/includes/filebackend/FileBackendMultiWrite.php +++ b/includes/filebackend/FileBackendMultiWrite.php @@ -92,7 +92,7 @@ class FileBackendMultiWrite extends FileBackend { * - noPushDirConts : (hack) Only apply directory functions to the master backend. * * @param array $config - * @throws MWException + * @throws FileBackendError */ public function __construct( array $config ) { parent::__construct( $config ); @@ -119,7 +119,7 @@ class FileBackendMultiWrite extends FileBackend { } $name = $config['name']; if ( isset( $namesUsed[$name] ) ) { // don't break FileOp predicates - throw new MWException( "Two or more backends defined with the name $name." ); + throw new FileBackendError( "Two or more backends defined with the name $name." ); } $namesUsed[$name] = 1; // Alter certain sub-backend settings for sanity @@ -129,20 +129,20 @@ class FileBackendMultiWrite extends FileBackend { $config['lockManager'] = 'nullLockManager'; // lock under proxy backend if ( !empty( $config['isMultiMaster'] ) ) { if ( $this->masterIndex >= 0 ) { - throw new MWException( 'More than one master backend defined.' ); + throw new FileBackendError( 'More than one master backend defined.' ); } $this->masterIndex = $index; // this is the "master" $config['fileJournal'] = $this->fileJournal; // log under proxy backend } // Create sub-backend object if ( !isset( $config['class'] ) ) { - throw new MWException( 'No class given for a backend config.' ); + throw new FileBackendError( 'No class given for a backend config.' ); } $class = $config['class']; $this->backends[$index] = new $class( $config ); } if ( $this->masterIndex < 0 ) { // need backends and must have a master - throw new MWException( 'No master backend defined.' ); + throw new FileBackendError( 'No master backend defined.' ); } } diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 2f3e0e2272..fe3a068456 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -965,7 +965,7 @@ abstract class FileBackendStore extends FileBackend { * * @param array $ops Same format as doOperations() * @return array List of FileOp objects - * @throws MWException + * @throws FileBackendError */ final public function getOperationsInternal( array $ops ) { $supportedOps = array( @@ -989,7 +989,7 @@ abstract class FileBackendStore extends FileBackend { // Append the FileOp class $performOps[] = new $class( $this, $params ); } else { - throw new MWException( "Operation '$opName' is not supported." ); + throw new FileBackendError( "Operation '$opName' is not supported." ); } } @@ -1091,7 +1091,7 @@ abstract class FileBackendStore extends FileBackend { // Perform the sync-only ops and build up op handles for the async ops... foreach ( $ops as $index => $params ) { if ( !in_array( $params['op'], $supportedOps ) ) { - throw new MWException( "Operation '{$params['op']}' is not supported." ); + throw new FileBackendError( "Operation '{$params['op']}' is not supported." ); } $method = $params['op'] . 'Internal'; // e.g. "storeInternal" $subStatus = $this->$method( array( 'async' => $async ) + $params ); @@ -1133,17 +1133,18 @@ abstract class FileBackendStore extends FileBackend { * to the order in which the handles where given. * * @param array $fileOpHandles - * @throws MWException + * @throws FileBackendError * @internal param array $handles List of FileBackendStoreOpHandle objects * @return array Map of Status objects */ final public function executeOpHandlesInternal( array $fileOpHandles ) { $section = new ProfileSection( __METHOD__ . "-{$this->name}" ); + foreach ( $fileOpHandles as $fileOpHandle ) { if ( !( $fileOpHandle instanceof FileBackendStoreOpHandle ) ) { - throw new MWException( "Given a non-FileBackendStoreOpHandle object." ); + throw new FileBackendError( "Given a non-FileBackendStoreOpHandle object." ); } elseif ( $fileOpHandle->backend->getName() !== $this->getName() ) { - throw new MWException( "Given a FileBackendStoreOpHandle for the wrong backend." ); + throw new FileBackendError( "Given a FileBackendStoreOpHandle for the wrong backend." ); } } $res = $this->doExecuteOpHandlesInternal( $fileOpHandles ); @@ -1157,12 +1158,12 @@ abstract class FileBackendStore extends FileBackend { /** * @see FileBackendStore::executeOpHandlesInternal() * @param array $fileOpHandles - * @throws MWException + * @throws FileBackendError * @return array List of corresponding Status objects */ protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { if ( count( $fileOpHandles ) ) { - throw new MWException( "This backend supports no asynchronous operations." ); + throw new FileBackendError( "This backend supports no asynchronous operations." ); } return array(); diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php index 82959d8564..4e03675510 100644 --- a/includes/filebackend/FileOp.php +++ b/includes/filebackend/FileOp.php @@ -74,7 +74,7 @@ abstract class FileOp { * * @param FileBackendStore $backend * @param array $params - * @throws MWException + * @throws FileBackendError */ final public function __construct( FileBackendStore $backend, array $params ) { $this->backend = $backend; @@ -83,7 +83,7 @@ abstract class FileOp { if ( isset( $params[$name] ) ) { $this->params[$name] = $params[$name]; } else { - throw new MWException( "File operation missing parameter '$name'." ); + throw new FileBackendError( "File operation missing parameter '$name'." ); } } foreach ( $optional as $name ) { diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php index dc2995fbfd..d79ceca174 100644 --- a/includes/filebackend/SwiftFileBackend.php +++ b/includes/filebackend/SwiftFileBackend.php @@ -124,7 +124,7 @@ class SwiftFileBackend extends FileBackendStore { public function __construct( array $config ) { parent::__construct( $config ); if ( !class_exists( 'CF_Constants' ) ) { - throw new MWException( 'SwiftCloudFiles extension not installed.' ); + throw new FileBackendException( 'SwiftCloudFiles extension not installed.' ); } // Required settings $this->auth = new CF_Authentication( @@ -837,12 +837,15 @@ class SwiftFileBackend extends FileBackendStore { * @param string $ts * @param int $format Output format (TS_* constant) * @return string - * @throws MWException + * @throws FileBackendError */ protected function convertSwiftDate( $ts, $format = TS_MW ) { - $timestamp = new MWTimestamp( $ts ); - - return $timestamp->getTimestamp( $format ); + try { + $timestamp = new MWTimestamp( $ts ); + return $timestamp->getTimestamp( $format ); + } catch ( MWException $e ) { + throw new FileBackendError( $e->getMessage() ); + } } /** -- 2.20.1